[WIP] Port build system to use only build.rs on unix #321

weiznich · 2016-10-25T10:37:42Z

Fix Build fails when there are spaces in the path to the project #320

weiznich · 2016-10-25T15:07:00Z

Any ideas why the windows builder is failing?

briansmith · 2016-10-25T16:32:08Z

Wow! Thanks for this!

Three questions to start with:

Is this code (or similar) being used in another open-source project? If so, which ones? It would help speed up the review if I can look at the history of the code, if there is any history.
How necessary is the target_build_utils dependency? Would it be a lot of work to do this without that dependency?
Does the gcc crate honor the CC, CXX, AS, etc. variables on Unix-ish hosts? In the current build system, CC and CXX are used to set the compiler, e.g. clang or GCC, or specific versions.

weiznich · 2016-10-25T18:16:05Z

Is this code (or similar) being used in another open-source project? If so, which ones? It would help speed up the review if I can look at the history of the code, if there is any history.

I've developed the code to fix the issue with the white spaces. So unfortunately there is no history for this.

How necessary is the target_build_utils dependency? Would it be a lot of work to do this without that dependency?

target_build_utils simplifies the handling of the target_triples. Especially it removes the parsing and unifying of those triples (For example it maps i586, i386 and i686 to x86). It should be possible to implement this without this crate, but it would result in quite a bit additional code that needs to be written. Furthermore it's only a build_dependency, which means its not added in the final library. Also target_build_utils and all of it's dependencies only contains pure rust code (as far as I can see). So it should build fine on every platform.

Does the gcc crate honor the CC, CXX, AS, etc. variables on Unix-ish hosts? In the current build system, CC and CXX are used to set the compiler, e.g. clang or GCC, or specific versions.

The gcc crate honors CC, CXX, AS, etc (and many more). It seems like it even detects if it's cross compiling to ios and sets the according sdk path. Take a look at the code for the compiler selection. We could also set the compiler manually using this function.

briansmith · 2016-10-25T21:35:26Z

@frewsxcv Do you have an opinion about this?

My main concerns about this kind of switch are:

Legibiility: Is the new thing as easy to read as the Makefile-based approach?
Performance for people hacking on ring: Is this going to slow down the build times for people who are actively hacking on ring, by forcing every C/asm file to be recompiled/regenerated every time anything changes? The Makefile-based approach allows incremental building of all the C/asm stuff?

Despite those concerns, I think it makes sense to find a way to make the build work without requiring GNU make.

briansmith · 2016-10-25T21:39:19Z

crypto/test/bn_test_lib.c

@@ -133,7 +133,7 @@ int BN_rand(BIGNUM *rnd, int bits, RAND *rng) {
  bit = (bits - 1) % 8;
  mask = 0xff << (bit + 1);

-  buf = OPENSSL_malloc(bytes);
+  buf = (uint8_t*) OPENSSL_malloc(bytes);


Why did you add this cast? The policy so far is to not cast (void *) since C doesn't require the cast, because in general we have a goal of eliminating every single cast.

I've tried to compile this as c++ code. In this case g++ complains that void* is no uint8_t*.
error: invalid conversion from ‘void*’ to ‘uint8_t*‘[-fpermissive]
It's not needed now, so I've removed this from the current version.

briansmith · 2016-10-25T21:40:09Z

crypto/constant_time_test.c

@@ -42,6 +42,9 @@
 * copied and put under another distribution licence
 * [including the GNU Public Licence.]
 */
+#ifdef __cplusplus
+extern "C" {
+#endif


If this change is important then please make it in a separate PR that explains why it was done. It shouldn't be needed, AFAICT.

briansmith · 2016-10-25T21:41:40Z

build.rs

+      "crypto/test/bn_test_lib.c",
+      "crypto/test/file_test.cc"];
+
+const C_FLAGS: &'static [&'static str] = &["-std=c1x",


Is this formatting the result of rustfmt? It seems strange to indent this so far to the right, especially considering the other things aren't indented this way.

It's produced by rustfmt in this way. 😞

briansmith · 2016-10-25T21:42:28Z

build.rs

+
+const AS_FLAGS: &'static [&'static str] = &["-Wa,--noexecstack,-gdwarf"];
+
+const IOS_MIN_VERSION: &'static str = "7.0";


I agree that all the stuff above, which are basically the options that control the build, should be at the top of this file. However, for the rest of the file, please organize things top-down: main, then build_msvc, then the rest of the stuff sorted in your best judgement in a top-down manner. (This is the style in ring in general.)

briansmith · 2016-10-25T21:44:39Z

build.rs

+    println!("cargo:rerun-if-changed=build.rs");
+}
+
+fn build_msvc(target_info: &TargetInfo, lib_path: PathBuf, disable_opt: bool) {


Please split the PR so that the build_msvc function is factored out, with the original logic, in a separate commit.

briansmith · 2016-10-25T21:50:31Z

build.rs

+        .chain(RING_ARM_SRCS.iter())
+        .chain(RING_X86_64_SRC.iter())
+        .chain(RING_X86_SRCS.iter())
+        .chain(RING_INTEL_SHARED_SRCS.iter()) {


Is this correct? It seems we also need to consider all the header files in all directories, and crypto/sha/asm/sha-x86_64.pl, in addition to the above.

It would be great if there were some way we could calculate the list of source files to track, and then traverse that list to do the actual build, instead of effectively calculating here and then also separately during the building.

For example, why not run through this iterator and call a "build_source" function that differentiates the type of source file by the extension, calling build_asm or whatever?

briansmith · 2016-10-25T21:54:12Z

build.rs

 }

 fn run_command_with_args<S>(command_name: S, args: &Vec<String>)
-                            where S: AsRef<std::ffi::OsStr> + Copy {
+    where S: AsRef<std::ffi::OsStr> + Copy


I guess this is due to rustfmt? If you want to reformat code you're not changing, I'll consider that, but please do that reformatting in a separate commit.

briansmith · 2016-10-25T22:05:18Z

build.rs

        panic!("{} execution failed", command_name.as_ref().to_str().unwrap());
    }
 }
+
+fn compile(file: &str, target_info: &TargetInfo, mut out_dir: PathBuf)


My understanding is that the gcc crate automatically parallelizes the build if you pass in multiple files, and so did the make-based approach. So, we need to decide whether we want to use the gcc crate's parallelism support, or whether we're going to (perhaps in a separate PR, and perhaps not necessarily you writing it) do the parallelism ourselves.

The benefit of doing the parallelism ourselves is that we can do it for the *.pl -> *.S and .S -> .o steps too. The downside to doing the parallelism ourselves is that it is more work than relying on the gcc crate's built-in stuff.

WDYT?

The gcc crate is not capable to distinguish between different kind of files. If I pass all RING_TEST_SRCS to the gcc crate I need to set the cpp flag. This means everything is compiled as c++ file. (The gcc crate simply calls CC or CXX on all passed files.) At this point some of the supported compilers are beginning to complain about incorrect compiler and linker flags. For example see this build.
Because of this I decided to compile every file on it's own. It should be simple to call this in parallel by using par_iter form rayon like the gcc crate.

briansmith · 2016-10-25T22:29:44Z

Thinking about this more:

The first step, I think, is agreeing on a plan. My main concern is whether we can (eventually) support incremental builds and parallelized builds. I'd like to hear your thoughts and make sure we agree on how that could eventually work, even if we don't do that in this PR.

After that, I'd like to see the PR broken up into at least these parts:

The changes to the C and C++ files, which should be their own PRs.
rustfmt on build.rs.
Factor out the build_msvc function, underneath main.
The RUST_SRCS and similar constants + generation of the cargo:rerun-if-changed output (on non-MSVC, at least; include Makefile and mk/*.mk).
The rest of the new stuff, underneath main, in the top-down style.

I would also be happy if #4 could be broken into multiple parts (e.g. dealing with the perlasm stuff in a separate commit), if that isn't an unreasonable amount of work.

Does this make sense to you?

weiznich · 2016-10-26T12:04:41Z

My main concern is whether we can (eventually) support incremental builds and parallelized builds.

The current version already checks if a file needs to be recompiled. (See the need_run method.). If someone changes a header file everything is recompiled.
For parallel builds we could simply use par_iter from rayon as mentioned above.

I've split the commit into smaller parts. Hopefully this is now easier to review.

briansmith · 2016-10-26T19:44:49Z

@weiznich I did the reformatting myself because it involves some rearranging the code to make it less horrific: 61e6afc.

briansmith

Thanks for making the changes. Here's another round of review. I didn't review everything carefully because I think there's going to be a lot of changes already.

briansmith · 2016-10-26T20:02:30Z

build.rs

+
+    fn find_msbuild_exe(program_files_env_var: &str,
+                        optional_amd64: Option<&str>)
+                        -> Result<std::ffi::OsString, ()> {


We never return Err(...) here, so the return type should be removed.

briansmith · 2016-10-26T20:02:49Z

build.rs

+                        disable_opt,
+                        &num_jobs,
+                        lib_path.clone(),
+                        out_dir));


We never return Err() so we should remove the try!(...).

I don't know how to configure rustfmt to format this in the compact style, but it should be like this:

build_msvc(&target_triple, disable_opt, &num_jobs, lib_path.clone(), out_dir);

briansmith · 2016-10-26T20:03:32Z

build.rs

@@ -65,7 +65,7 @@
 )]

 use std::env;
-use std::path::Path;
+use std::path::{Path, PathBuf};


There are some uses of std::path::PathBuf below that should have the std::path:: qualification removed.

briansmith · 2016-10-26T20:11:29Z

build.rs

-        let configuration = if disable_opt { "Debug" } else { "Release" };
-        let args = vec![
+    let configuration = if disable_opt { "Debug" } else { "Release" };
+    let args = vec![
            format!("/m:{}", num_jobs),
            format!("/p:Platform={}", platform),
            format!("/p:Configuration={}", configuration),
            format!("/p:OutRootDir={}/", out_dir),
        ];


These lines are too indented. (When I reduce the indention by 4 spaces in my copy and then run rustfmt it keeps it as-is.)

briansmith · 2016-10-26T20:24:37Z

build.rs

 const LIB_NAME: &'static str = "ring";

+const RING_SRC: &'static [&'static str] = &["crypto/aes/aes.c",


I can't figure out how to make rustfmt do something reasonable with this. Let's format all of these variables containing lists of files like RING_X86_64_SRC and add #[cfg_attr(rustfmt, rustfmt_skip)] to each of them.

briansmith · 2016-10-26T20:38:00Z

build.rs

+      "crypto/test/file_test.h",
+      "crypto/test/bn_test_util.h"];
+
+const RING_PERL_INCLUDES: &'static [&'static str] = &["crypto/sha/asm/sha-x86_64.pl"];


crypto/sha/asm/sha-armv8.pl is another thing that should be added to this list.

briansmith · 2016-10-26T20:46:11Z

build.rs

+                                           "-Wnested-externs",
+                                           "-Wstrict-prototypes"];
+
+const CPP_FLAGS: &'static [&'static str] = &["-std=c++0x",


I noticed you copied CFLAGS and CPPFLAGS from mk/top_of_makefile.mk, but you didn't copy CXXFLAGS. Instead of looks like you confused CPPFLAGS and CXXFLAGS. CPPFLAGS should be passed to every compilation (*.S, *.c, *.cc), CFLAGS should additionally be passed for *.c only, and CXXFLAGS should be passed for *.cc.

In particular, -std=c++0x should be in CXXFLAGS but most of the other stuff here should remain in CPPFLAGS.

briansmith · 2016-10-26T20:54:38Z

build.rs

+    let test_header_change = RING_TEST_HEADERS.iter()
+        .map(Path::new)
+        .any(|p| need_run(&p, test_target)) ||
+                             lib_header_change;


I suggest that we just check if any of find crypto include mk -name "*.h" -o -name "*.inl" -o -name "*.mk" changed, and if so, rebuild all the libraries. Especially since the test lib will go away sometime soon.

briansmith · 2016-10-26T20:56:49Z

build.rs

+        }
+        c.compile("libring-core.a");
+    }
+


Please factor out the duplicate logic between the code in this function above this line and the code below this line.

briansmith · 2016-10-26T20:58:33Z

build.rs

@@ -370,6 +464,45 @@ fn run_command_with_args<S>(command_name: S, args: &Vec<String>)
    }
 }

+fn make_asm(source: &str, mut dst: PathBuf, target_info: &TargetInfo)
+            -> String {
+    let p = Path::new(source);


Here we need to consider crypto/perlasm/*.pl and crypto/sha/{sha-armv8.pl, sha-x86_64.pl} the same way we consider *.h and *.inl in the compile() function.

briansmith · 2016-10-26T21:14:25Z

Are you the copyright holder for these changes, or is there an employer or somebody else that owns the copyright? If somebody else, who?

Please add the contributor statement to the bottom of each commit; See https://github.com/briansmith/ring#contributing.

If you add the contributor statement to the commit that factors out the build_msvc() function then I'll commit it to master while we work on the rest.

weiznich · 2016-10-26T22:44:25Z

I've updated the pull request according to your comments.

briansmith · 2016-10-27T00:17:40Z

Thanks!

I landed the "Move msvc build logic to own function" commit as a0ee4b5, tweaked as I suggested above.

briansmith · 2016-10-27T00:37:46Z

@weiznich Just to make sure you're aware, I also wrote many comments above that are now hidden behind the "show outdated" links.

coveralls · 2016-10-27T03:45:02Z

Coverage remained the same at 90.794% when pulling 308502e77328cd427ad7e3b02e71ae8b944a04ec on weiznich:fix_320 into b4b084e on briansmith:master.

coveralls · 2016-10-27T03:45:02Z

Coverage remained the same at 90.794% when pulling 308502e77328cd427ad7e3b02e71ae8b944a04ec on weiznich:fix_320 into b4b084e on briansmith:master.

coveralls · 2016-10-27T06:39:24Z

Coverage remained the same at 90.794% when pulling 308502e77328cd427ad7e3b02e71ae8b944a04ec on weiznich:fix_320 into b4b084e on briansmith:master.

weiznich · 2016-10-27T14:03:52Z

@weiznich Just to make sure you're aware, I also wrote many comments above that are now hidden behind the "show outdated" links.

Thanks, I've missed them.

The current version should address those comments. Also I added a commit which introducing parallel compilation of the native stuff. But I'm not sure if this improves the compiletime.

briansmith · 2017-02-16T02:55:34Z

Cargo.toml

@@ -266,6 +266,11 @@ name = "ring"
 libc = "0.2.20"
 untrusted = "0.3.2"

+[build-dependencies]
+gcc = "0.3"
+target_build_utils = "0.2"


Please change the target_build_dependencies dependency to { version = "0.2", default-features = false } to reduce the dependencies.

I filed #457 for removing the target_build_utils dependency after this PR is merged. The target_build_utils itself is about 10 additional transitive dependencies, which is too much long-term. It's OK for now, in order to make forward progress.

briansmith · 2017-02-16T02:57:41Z

Cargo.toml

@@ -266,6 +266,11 @@ name = "ring"
 libc = "0.2.20"
 untrusted = "0.3.2"

+[build-dependencies]
+gcc = "0.3"


We don't use gcc = { version = "0.3", features = ["parallel"] } because we do the parallelism ourselves, right? If that is the case, let's add a comment to that effect here.

briansmith · 2017-02-16T08:06:00Z

In travis.sh, we run "make --version". That should be removed.

briansmith

I've reviewed up to, but not including, the PerlAsm changes. I'll make the iOS and macOS changes myself on top of your changes.

briansmith · 2017-02-16T03:48:30Z

build.rs

@@ -15,7 +15,7 @@
 // TODO: Deny `unused_qualifications` after
 // https://github.com/rust-lang/rust/issues/37345 is fixed.
 #![allow(
-    box_pointers, // TODO
+// TODO


Remove the TODO comment.

briansmith · 2017-02-16T03:49:25Z

mk/top_of_makefile.mk

-else ifeq ($(CMAKE_BUILD_TYPE),RELEASE)
-CPPFLAGS += -DNDEBUG -O3
-else ifeq ($(CMAKE_BUILD_TYPE),RELWITHDEBINFO)
-CPPFLAGS += -DNDEBUG -O3


This is the most serious issue so far: You didn't copy the logic for NDEBUG into the new build system. In particular, in non-debug builds, we need to add -DNDEBUG to CPPFLAGS so that the assertions are not compiled in.

(It seems gcc-rs automatically adds -O3 or -O0 depending on whether or not --release is passed on the command line, so you don't need to do anything with -O, just the NDEBUG logic.)

-DNDEBUG is now added if OPT_LEVEL != "0" otherwise -DDEBUG is added

briansmith · 2017-02-16T10:02:48Z

build.rs

+      "-Wnested-externs",
+      "-Wstrict-prototypes"];
+
+const CXX_FLAGS: &'static [&'static str] = &["-std=c++0x"];


Add a comment: // GCC 4.6 requires "c++0x" instead of "c++11".

briansmith · 2017-02-16T10:03:13Z

build.rs

+
+#[cfg_attr(rustfmt, rustfmt_skip)]
+const C_FLAGS: &'static [&'static str] =
+    &["-std=c1x",


Add a comment: // GCC 4.6 requires "c1x" instead of "c11".

briansmith · 2017-02-16T10:09:36Z

build.rs

+                    let _ = c.flag(f);
+                }
+                if target_info.target_os() != "none" {
+                    let _ = c.flag("-fstack-protector");


I think you confused "CPPFLAGS" and "CXXFLAGS". This should be done for both "c" and "cc". Here's the logic you need to implement:

ifneq ($(filter-out none redox,$(TARGET_SYS)),) CPPFLAGS += -fstack-protector endif

briansmith · 2017-02-16T10:15:56Z

build.rs

+                let _ = match (target_info.target_os(),
+                               target_info.target_arch()) {
+                    ("macos", _) => c.flag("-gfull"),
+                    _ => c.flag("-g3"),


This needs to be done for both "c" and "cc". (Again, I think you confused CXXFLAGS and CPPFLAGS.)

briansmith · 2017-02-16T10:17:38Z

build.rs

+                }
+                let _ = match (target_info.target_os(),
+                               target_info.target_arch()) {
+                    ("macos", _) => c.flag("-gfull"),


Copy the comment over: // ``-gfull`` is required for Darwin's |-dead_strip|.

briansmith · 2017-02-16T10:18:34Z

mk/top_of_makefile.mk

-LDFLAGS += -Wl,--gc-sections
-endif
-
-ASFLAGS += -Wa,--noexecstack,-gdwarf


The ASFLAGS logic wasn't copied over.

ASFLAGS is not copied because it seems to be not used (As far as I can tell from the output of a Makefile based build)

briansmith · 2017-02-16T10:34:34Z

build.rs

+                    a.extend(b.into_iter());
+                    a
+                });
+    if objs.par_iter()


Please add a comment here about what this does. I think it would be something like "Rebuild the library if necessary."

briansmith · 2017-02-16T10:35:39Z

build.rs

+                    header_changed: bool)
+    where P: ParallelIterator<Item = String>
+{
+    let objs = additional.chain(lib_src.par_iter().map(|a| String::from(*a)))


Let's add a comment here, e.g. "Compile all the (dirty) source files into object files."

briansmith · 2017-02-16T10:42:00Z

When you make the next set of changes, please:

Rebase the entire series on top of the current master.
Make your changes that address the review comments as 1 or more separate commit(s), leaving the existing commits unchanged.

In particular, if you rebase on top of the current master then the build script output will be in the Travis CI logs because I just committed a change to run cargo build and cargo test with the -vv option.

briansmith · 2017-02-16T10:59:30Z

Here's the process I'm using to review the changes related to compiler flags:

First I pick an arbitrary target, e.g. 64-bit Linux.

I run cargo test -vv --features=rsa_signing on current master to get a build log. Then I run cargo test -vv --features=rsa_signing on your branch to get a build log.

Then I find the command line for compiling an arbitrary C file, e.g. crypto.c, in each build log. I replace spaces with newlines, sort the lines, and then diff the two commands to see what flags are different between master and your PR.

Then I find a command line for compiling an arbitrary C++ file, e.g. bn_test.cc, and do the same diffing procedure. Similarly, tomorrow I will do the same for an arbitrary perl invocation, for an arbitrary assembly invocation, and for each linking/librarian invocation.

In addition to all the above, I've inspected the Makefiles, looking in particular for comments that explain important design decisions. Hopefully I've asked you to copy all the important comments in my current review comments. (There are a bunch of TODO comments that you don't need to worry about; I've captured them in a separate document.)

Then I will repeat the whole procedure for 32-bit Linux. And then 64-bit macOS, then 64-bit iOS, then 32-bit iOS, then 64-bit Android, then 32-bit Android.

Things have to be reviewed this way, instead of comparing the old Makefile logic to the new build.rs code, because (1) it is too hard to compare them exactly, and (2) the gcc crate adds its own additional command line parameters that aren't obvious.

So, it is slow going but it is getting done.

briansmith · 2017-02-16T11:00:34Z

Oh, another thing I forgot in the previous comment: It is important not only to run cargo test -vv --features=rsa_signing, but also we must run the same commands with and without --release, because the release configuration is different. In particular, this is how I found the NDEBUG issue.

* Fix briansmith#320 I agree to license my contributions to each file under the terms given at the top of each file I changed.

I agree to license my contributions to each file under the terms given at the top of each file I changed.

* Track all source, header and make files I agree to license my contributions to each file under the terms given at the top of each file I changed.

I agree to license my contributions to each file under the terms given at the top of each file I changed.

weiznich · 2017-02-16T18:13:51Z

I've rebased everything on master and addressed the comments. I had no time to do any in depth comparison between the output of the old and the new build scripts.

coveralls · 2017-02-17T04:17:37Z

Changes Unknown when pulling 1d2939f on weiznich:fix_320 into ** on briansmith:master**.

briansmith · 2017-02-23T02:05:20Z

Still working on this. At this point there's no reason to update this PR any further. I've got a private branch that builds on this PR to get the macOS and Windows stuff working. Hopefully done in the next few days.

briansmith · 2017-03-06T05:35:11Z

Thanks very much for contributing this! I landed a modified version of this in this series of commits:

86c24b9 Use only build.rs to build the native libraries for non-Windows builds.
fa55e87 Pass out_dir by moving it in build.rs.
55ac79c Fix Windows build.
f5fc35a Build native libraries in parallel.
5a50c3c Remove Makefiles, which are now unused.
f72d7c4 Only run the build script if something changed.
65eeba2 Remove redundant -fpic C/C++ compiler option.

This seems to have been the fix for the macOS build failures:

5efd2ae Pass -stdlib=c++ to Clang for Apple targets.

I also extended your PR to generate more output:
f66d641 Print command line for every command executed in build.rs like gcc-rs.

I also cleaned up some stuff (I actually pushed most of my cleanup stuff in commits earlier in the week):
5b4e29a Stop defining BORINGSSL_IMPLEMENTATION.

I also removed the target_build_utils dependency. It simply added too many dependencies and slowed down a clean build way too much. AFAICT all of its functionality is subsumed by recent versions of Cargo with the CARGO_CFG_* variables. That probably wasn't true when you originally wrote the PR though. Here's that commit:
5c77732 Stop using target_build_utils.

Finally, because Windows is the primary platform I develop ring on, it is important to use the new build system on Windows right away, so I moved Windows over as well:
cfdc44f Generalize new PerlAsm logic in build.rs to work for -msvc, others.
b2ed266 Stop using MSBuild on Windows.
0c8cb95 Add more Visual-Studio-related stuff to .gitignore.

It's unlikely that things are perfect yet. I bet we broke some of the platforms we don't have on Travis CI like Redox (cc @jackpot51) and/or OpenBSD (cc @tatsuya6502) and/or iOS (cc @kali). My policy is to treat platforms that we don't have CI coverage for as "Tier 2" in that we don't back out changes that break them, but we accept bug fixes to get them working again, and we notify relevant people when we suspect we might have broken them.

And, I still need to go through the Windows build and add back some inessential compilation flags. I'll do that in the next few days.

Thanks again for the tremendous contribution here!

kali · 2017-03-06T09:09:54Z

Wow, nice cleanup! I can confirm the tests are passing on ios on a real phone, compiled for armv7 and aarch64.

Building for the ios simulator is not workign though. I'll open a separate issue for that.

weiznich force-pushed the fix_320 branch 2 times, most recently from 4fa1148 to 3dd68d3 Compare October 25, 2016 14:01

weiznich mentioned this pull request Oct 25, 2016

Build fails when there are spaces in the path to the project #320

Closed

weiznich force-pushed the fix_320 branch from 3dd68d3 to 3fa41bf Compare October 25, 2016 18:18

briansmith reviewed Oct 25, 2016

View reviewed changes

briansmith requested changes Oct 25, 2016

View reviewed changes

briansmith mentioned this pull request Oct 26, 2016

Observe CARGO_CFG_TARGET_* in build.rs #322

Closed

weiznich force-pushed the fix_320 branch from 3fa41bf to 269c4f1 Compare October 26, 2016 11:39

briansmith requested changes Oct 26, 2016

View reviewed changes

weiznich force-pushed the fix_320 branch 2 times, most recently from 1f5eadf to 308502e Compare October 26, 2016 22:42

weiznich force-pushed the fix_320 branch 2 times, most recently from ffab358 to 8bc2e0a Compare October 27, 2016 13:55

weiznich force-pushed the fix_320 branch from 8bc2e0a to 4498526 Compare October 27, 2016 14:06

briansmith reviewed Feb 16, 2017

View reviewed changes

This was referenced Feb 16, 2017

Remove target_build_utils dependency #457

Closed

Improve use of -march, -mtune, and -mabi C compiler options #458

Closed

briansmith requested changes Feb 16, 2017

View reviewed changes

weiznich added 5 commits February 16, 2017 12:50

Use only build.rs on unix to build the native libraries

9e947a3

* Fix briansmith#320 I agree to license my contributions to each file under the terms given at the top of each file I changed.

Build native libraries in parallel

f117dc2

I agree to license my contributions to each file under the terms given at the top of each file I changed.

We don't need the makefiles anymore

a36243b

I agree to license my contributions to each file under the terms given at the top of each file I changed.

Add logic to only run the build script if anything has changed

7787e36

* Track all source, header and make files I agree to license my contributions to each file under the terms given at the top of each file I changed.

Minor build script improvements

1d2939f

I agree to license my contributions to each file under the terms given at the top of each file I changed.

weiznich force-pushed the fix_320 branch from 0fe75f1 to 1d2939f Compare February 16, 2017 18:10

briansmith modified the milestones: 0.7, 0.7.n Feb 18, 2017

briansmith closed this Mar 6, 2017

kali mentioned this pull request Mar 6, 2017

Ios simuator and new build #475

Closed

briansmith modified the milestones: 0.7.n, 0.7.3 Mar 20, 2017


		const AS_FLAGS: &'static [&'static str] = &["-Wa,--noexecstack,-gdwarf"];

		const IOS_MIN_VERSION: &'static str = "7.0";

		const LIB_NAME: &'static str = "ring";

		const RING_SRC: &'static [&'static str] = &["crypto/aes/aes.c",

[WIP] Port build system to use only build.rs on unix #321

[WIP] Port build system to use only build.rs on unix #321

Conversation

weiznich commented Oct 25, 2016

weiznich commented Oct 25, 2016

briansmith commented Oct 25, 2016

weiznich commented Oct 25, 2016 • edited Loading

briansmith commented Oct 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briansmith Oct 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briansmith commented Oct 25, 2016

weiznich commented Oct 26, 2016

briansmith commented Oct 26, 2016

briansmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briansmith commented Oct 26, 2016

weiznich commented Oct 26, 2016

briansmith commented Oct 27, 2016

briansmith commented Oct 27, 2016

coveralls commented Oct 27, 2016

coveralls commented Oct 27, 2016

coveralls commented Oct 27, 2016 • edited Loading

weiznich commented Oct 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briansmith commented Feb 16, 2017

briansmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briansmith commented Feb 16, 2017

briansmith commented Feb 16, 2017

briansmith commented Feb 16, 2017

weiznich commented Feb 16, 2017

coveralls commented Feb 17, 2017 • edited Loading

briansmith commented Feb 23, 2017

briansmith commented Mar 6, 2017

kali commented Mar 6, 2017

weiznich commented Oct 25, 2016 •

edited

Loading

briansmith Oct 25, 2016 •

edited

Loading

coveralls commented Oct 27, 2016 •

edited

Loading

coveralls commented Feb 17, 2017 •

edited

Loading